Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optional trait type #1786

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

k2bd
Copy link
Contributor

@k2bd k2bd commented Apr 10, 2024

Closes #1298

This PR:

  • Creates the Optional trait type as a shorthand for Union(None, ...)
  • Changes exception messages in Union to refer to subclass names, e.g. Optional (let me know if this is controversial and I can change!)
  • Adds xfail tests for Constant, Union, and Optional demonstrating behaviour discussed in Constant constraint not enforced in Union #1784, some of these may also be controversial and I can remove/alter them if so

Checklist

  • Tests
  • Update API reference (docs/source/traits_api_reference)
  • Update User manual (docs/source/traits_user_manual)
  • Update type annotation hints in stub files

@k2bd
Copy link
Contributor Author

k2bd commented Apr 23, 2024

Sorry if I'm missing something obvious, I'm not sure I can link the error in tests to these changes. Additionally the failing test passes for me on my Ubuntu-on-WSL-on-Windows.

Update: from here I'm guessing it's an external (to this MR) issue

@mdickinson
Copy link
Member

Sorry, @k2bd: there hasn't been much in the way of spare cycles for Traits maintenance recently. Thank you for the PR - I do hope to get to it soon.

Yes, there's at least one unrelated issue here, related to a new release of PySide 6. I'll investigate.

@k2bd
Copy link
Contributor Author

k2bd commented Apr 24, 2024

@mdickinson no worries at all! I just got around to looking into the error yesterday after seeing it wasn't something immediately apparent at the time, and didn't want to keep anyone waiting too long if it was a subtle problem with what I wrote!

At the time, I couldn't find other workflow failures I had access to to see it was likely independent

@mdickinson
Copy link
Member

I've opened #1787 for the current crop of CI failures. This looks like a TraitsUI + PySide 6.7.0 issue.

@k2bd k2bd force-pushed the feat/1298-optional-traits branch from eda60d4 to 6c58d95 Compare April 29, 2024 08:24
@k2bd k2bd force-pushed the feat/1298-optional-traits branch from 6c58d95 to 13fcc59 Compare May 26, 2024 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: "Optional(Foo)" as a synonym for "Union(None, Foo)"
2 participants